fix: Camera state management #990

M123-dev · 2022-01-20T20:38:50Z

Yet another fix for the camera + some further extractions

monsieurtanuki

Hi @M123-dev!
Good job, but with some renaming it could be much better - cf. my comments.

packages/smooth_app/lib/pages/scan/scanner_state_manager.dart

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart

M123-dev · 2022-01-21T11:36:02Z

Thanks for your review @monsieurtanuki, applied your suggestions

monsieurtanuki

Thank you @M123-dev, and sorry about the following remarks:

please rename the file too, something like lifecycle_manager.dart
I took the names from Android lifecycle, so it's either (onStart and onStop) or (onResume and onPause). I suggest (onResume and onPause) in order to avoid the question "is there an onRestart too?"

jasmeet0817 · 2022-01-21T15:07:14Z

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart

    final CameraDescription camera = cameras[_cameraIndex];
-    _controller = CameraController(
+
+    final CameraController cameraController = CameraController(
      camera,
      ResolutionPreset.high,


have we tested other resolutions ? does it influence the scanning a lot ? it could alleviate some of the resource concerns

No not tested in any way, just took high that we have a "high" resolution, we fill nearly the full screen with the preview but not max so that we don't spend too much performance. After all it's just the preview not how much we get from the camera which very likely needs more performance.

jasmeet0817 · 2022-01-21T15:08:08Z

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart

-        return;
+    _controller = cameraController;
+
+    // If the controller is updated then update the UI.


out of curiousity, when does the listener fire, in other words, what does "controller is updated" mean?

When I am right this is used to update the page from showing a loading indicator to display the camera preview.

ok we should confirm that, because it's rerendering the widget.

jasmeet0817 · 2022-01-21T15:09:27Z

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart

+    _controller = cameraController;
+
+    // If the controller is updated then update the UI.
+    cameraController.addListener(() {


we keep adding this listener every time _startLiveFeed is called, shouldn't we do it just once?

We only call _startLiveFeed once (and again but then the controller is already disposed) so it shouldn't matter.

we also call _startLiveFeed from LifeCycleManager.onResume, so in the two cases you described below.

jasmeet0817 · 2022-01-21T15:11:30Z

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart

+      child: Center(
+        key: ValueKey<bool>(stoppingCamera),
+        child: CameraPreview(
+          _controller!,
        ),
      ),
    );
  }

  Future<void> _startLiveFeed() async {


out of curiosity when is _startLiveFeed called?

In initState after checking which camera to use.

jasmeet0817

PTAL, have lots of questions

jasmeet0817 · 2022-01-21T15:15:56Z

Yet another fix for the camera + some further extractions

What does it fix ?

jasmeet0817 · 2022-01-21T15:25:01Z

packages/smooth_app/lib/pages/scan/scanner_state_manager.dart

+    if (state == AppLifecycleState.inactive) {
+      widget.onStop.call();
+    } else if (state == AppLifecycleState.resumed) {
+      widget.onResume.call();


is it possible for you to explain in comments when AppLifeCycleState and when onVisibilityChanged is called, both call onStop or onResume and right now as a reader I don't understand changes on the camera on device trigger what...

For short, the visibility is if the scanner is currently displayed. So if it is the currently selected tab and it is also triggered if another page like search or the product page is above it.

The life cycle status is from the whole app. So this is triggered when the app is minimized and put back into focus again (useful for #982).

The two functions are just stopping and restarting the camera.

M123-dev · 2022-01-21T21:19:58Z

Yet another fix for the camera + some further extractions

What does it fix?

@jasmeet0817

There were some minor problems with disposing of the camera.

When going to another page or closing the camera it only stopped the image stream and not disposed the camera so for the system it was still active. (This is what caused #982)

The extraction part is just moving the life cycle management into its own widget.

Before:

ScanPage
-ML
--Overlay+life cycle

-QR Scanner
--Overlay+life cycle

Now:

ScanPage
-Overlay

-ML
--life cycle

-QR Scanner
--life cycle

The life cycle has to be below the scanners since we pass the stop and start methods.

M123-dev · 2022-01-21T21:29:03Z

Thanks for your review @jasmeet0817 and @monsieurtanuki you are right some renaming and commenting would be useful.

I, unfortunately, can't work on it until Monday if anyone of you wants to add those things feel free to do so that we can try the changes a bit faster.

monsieurtanuki

@M123-dev I'll take care of the renaming.

commit 738fdf9 Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sun Jan 23 16:43:17 2022 +0100 autolabeler-issue-improvements (#1001) autolabeler-issue-improvements commit 5fe7abc Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 21:07:51 2022 +0100 Update issue-labeler-v1.yml commit 9b227cb Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 21:06:22 2022 +0100 Update issue-labeler-v1.yml commit 20fce2e Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 21:03:22 2022 +0100 Update and rename issue-labeler.yml to issue-labeler-v1.yml commit 63ae13a Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 21:01:45 2022 +0100 Update issue-label.yml commit a1051cc Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 20:58:39 2022 +0100 Rename issue-autolabel.yml to issue-labeler.yml commit bf977a5 Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 20:56:19 2022 +0100 Create issue-label.yml (#1000) * Create issue-label.yml * Create issue-autolabel.yml commit 8adcc60 Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 20:50:55 2022 +0100 Update labeler.yml commit 8cbca14 Author: Moises <50844424+gatodenegocios@users.noreply.github.com> Date: Sat Jan 22 13:13:08 2022 -0300 fix: bug "Smooth Simple Button" in "SmoothAlertDialogue". issue #993 (#994) * feat: 🎸 create 'smooth action button' component Specifc button to use in the 'smooth alert dialog'. Created because the 'smooth simple button' creates a render box problem if multiple of them are in a horizontal group. * refactor: 💡 change the action buttons type requirement change from 'smooth simple button' to 'smooth action button' * docs: ✏️ change the action buttons type requirement in the doc * refactor: 💡 change the buttons in the smooth alert dialogues change the buttons to the new type of button requirement * refactor: 💡 remove unused imports * Rename smoot_action_button.dart to smooth_action_button.dart * Update packages/smooth_app/lib/pages/user_preferences_profile.dart * Update packages/smooth_app/lib/pages/product/common/product_dialog_helper.dart * Update packages/smooth_app/lib/pages/product/common/product_list_dialog_helper.dart * Update packages/smooth_app/lib/pages/user_management/sign_up_page.dart * Update packages/smooth_app/lib/pages/user_preferences_settings.dart * Update packages/smooth_app/lib/views/bottom_sheet_views/user_contribution_view.dart * Update packages/smooth_ui_library/lib/dialogs/smooth_alert_dialog.dart * Update packages/smooth_app/lib/widgets/loading_dialog.dart * Update packages/smooth_app/lib/pages/product/nutrition_page_loaded.dart Co-authored-by: Moises Alonso <moises.alonso@codeminer42.com> Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com> commit d1194ad Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat Jan 22 17:10:48 2022 +0100 chore: New Crowdin translations to review and merge (#997) * New Crowdin translations by Github Action (#677) Co-authored-by: Crowdin Bot <support+bot@crowdin.com> * chore: New Crowdin translations Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Crowdin Bot <support+bot@crowdin.com> Co-authored-by: Pierre Slamich <pierre@openfoodfacts.org> Co-authored-by: M123-dev <marvinmoel05@gmail.com> commit 6f20c50 Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Sat Jan 22 16:09:57 2022 +0100 Update dependabot.yml commit 3d4bc59 Author: monsieurtanuki <fabrice_fontaine@hotmail.com> Date: Sat Jan 22 11:11:28 2022 +0100 fix: lifecycle refactoring (#995) Renamed file: * `lifecycle_manager.dart`: was `scanner_state_manager.dart`; renamed `onStop` in `onPause` Impacted files: * `continuous_scan_page.dart`: refactored * `labeler.yaml`: refactored * `ml_kit_scan_page.dart`: refactored commit 72a2f27 Author: Marvin M <39344769+M123-dev@users.noreply.github.com> Date: Sat Jan 22 10:54:27 2022 +0100 fix: Camera state management (#990) * fix: Camera state management * Extracted lifecycle to own ScannerStateManager * Update ml_kit_scan_page.dart * Review commit ecdc37f Author: Pierre Slamich <pierre@openfoodfacts.org> Date: Fri Jan 21 20:02:39 2022 +0100 tag-picker-autolabeler (#988) * tag-picker-autolabeler tag-picker-autolabeler * Update labeler.yml * Update labeler.yml commit 4f7c448 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Jan 21 14:25:49 2022 +0100 chore: New Crowdin translations to review and merge (#992) * New Crowdin translations by Github Action (#677) Co-authored-by: Crowdin Bot <support+bot@crowdin.com> * chore: New Crowdin translations Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Crowdin Bot <support+bot@crowdin.com> Co-authored-by: Pierre Slamich <pierre@openfoodfacts.org> Co-authored-by: M123-dev <marvinmoel05@gmail.com> commit 692de25 Author: jasmeet0817 <jasmeet0817@gmail.com> Date: Thu Jan 20 21:48:53 2022 +0100 Add new product addition page [partial impl] (#989) commit 954449e Author: monsieurtanuki <fabrice_fontaine@hotmail.com> Date: Thu Jan 20 20:28:07 2022 +0100 feat: #935 - now we display localized (downloaded) data during onboarding (#986) Deleted files: * `sample_eco_knowledge_panels.json`: replaced by new file `sample_knowledge_panels.json` * `sample_health_knowledge_panels.json`: replaced by new file `sample_knowledge_panels.json` New files: * `abstract_onboarding_data.dart`: Abstraction of data we download, store and reuse at onboarding. * `dao_string.dart`: Where we store strings. * `loading_dialog.dart`: Dialog with a stop button, while a future is running. * `onboarding_data_knowledge_panels.dart`: Helper around knowledge panels we download, store and reuse at onboarding. * `onboarding_data_product.dart`: Helper around a product we download, store and reuse at onboarding. * `onboarding_loader.dart`: Helper around data we download, store and reuse at onboarding. * `sample_knowledge_panels.json`: json data downloaded today for en_US, cf. onboarding_data_knowledge_panels.dart * `tmp.dart`: Fixes to off-dart. Impacted files: * `knowledge_panel_page_template.dart`: added parameters to 1. use the database (and downloaded data) and 2. limit the display to a given panel * `knowlegde_panel_builder.dart`: added a method to display only one panel * `local_database.dart`: added new dao `DaoString` * `new_product_page.dart`: refactored * `next_button.dart`: added an optional call to async methods at "next" time - e.g. data download * `nutrition_page_loaded.dart`: now uses the new `LoadingDialog` * `onboarding_flow_navigator.dart`: now using the database to get more relevant (previoulsy downloaded) data * `preferences_page.dart`: now using previously downloaded data instead of assets * `product_dialog_helper.dart`: now uses the new `LoadingDialog` * `question_card.dart`: unrelated minor refactoring * `sample_eco_card_page.dart`: now using previously downloaded data instead of assets, and displaying only the environment card * `sample_health_card_page.dart`: now using previously downloaded data instead of assets, and displaying only the health card * `sample_product_json.dart`: json data downloaded today for en_US, cf. onboarding_data_product.dart * `sign_up_page.dart`: now uses the new `LoadingDialog` commit fe682a6 Author: Moises <50844424+gatodenegocios@users.noreply.github.com> Date: Wed Jan 19 13:49:00 2022 -0300 feat: sign out confirmation step. Issue: #884 (#979) * feat: 🎸 add a confirmation step to log out * fix: 🐛 Change the 'log out' term to 'sign out' * refactor: 💡 Change the alert dialogue to smooth alert dialog * refactor: 💡 removing unused variable 'theme' * refactor: 💡 specifying widgets on SmoothAlertDialog actions Co-authored-by: Moises Alonso <moises.alonso@codeminer42.com> commit 6a166aa Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed Jan 19 12:17:47 2022 +0100 chore: New Crowdin translations to review and merge (#978) * New Crowdin translations by Github Action (#677) Co-authored-by: Crowdin Bot <support+bot@crowdin.com> * chore: New Crowdin translations * Update packages/smooth_app/lib/l10n/app_es.arb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Crowdin Bot <support+bot@crowdin.com> Co-authored-by: Pierre Slamich <pierre@openfoodfacts.org> Co-authored-by: M123-dev <marvinmoel05@gmail.com> commit 76f29c8 Author: monsieurtanuki <fabrice_fontaine@hotmail.com> Date: Wed Jan 19 10:09:36 2022 +0100 feat: #931 - displaying the history in dialog instead of log print (#983) Impacted files: * `smooth_app/pubspec.yaml`: upgraded * `smooth_ui_library/pubspec.yaml`: upgraded * `smooth_app/pubspec.lock`: impacted by yaml upgrade * `smooth_ui_library/pubspec.lock`: impacted by yaml upgrade * `smooth_ui_library/example/pubspec.lock`: impacted by yaml upgrade * `user_preferences_dev_mode.dart`: displaying the history in dialog instead of log print commit e7ebbb7 Author: monsieurtanuki <fabrice_fontaine@hotmail.com> Date: Wed Jan 19 09:48:30 2022 +0100 feat: #880 - cosmetic changes and refactoring (#985) Impacted files: * `app_en.arb`: added a label for "unknown match" * `app_fr.arb`: added a label for "unknown match" * `personalized_ranking_page.dart`: added style for selected/unselected tab; added label for "unknown match"; added label even when empty list; now using new enum `MatchTab` in order to prevent confusion between indices * `smooth_it_model.dart`: added enum `MatchTab` and refactored accordingly commit e145883 Author: monsieurtanuki <fabrice_fontaine@hotmail.com> Date: Tue Jan 18 19:13:57 2022 +0100 feat: #880 - ranking page similar to mock-ups (#984) Impacted files: * `app_en.arb`: added 3 labels * `app_fr.arb`: added 3 labels * `personalized_ranking_page.dart`: reordered the tabs; now using tabs instead of bottom navigation bar; added optional subtitle * `smooth_it_model.dart`: removed now useless method and field commit 8cb5529 Author: jasmeet0817 <jasmeet0817@gmail.com> Date: Tue Jan 18 16:09:44 2022 +0100 Set min length for columns in a row (#980) * Scan example onboarding page: Switch svgs of ecoscore and nutriscore * Set min length for columns in a row * Format Co-authored-by: Jasmeet Singh <jasmeetsingh@google.com>

M123-dev added 3 commits January 20, 2022 19:51

fix: Camera state management

32f3f44

Extracted lifecycle to own ScannerStateManager

04b49c2

Update ml_kit_scan_page.dart

059fd12

M123-dev added the scanning label Jan 20, 2022

M123-dev requested review from jasmeet0817 and monsieurtanuki January 20, 2022 20:38

github-actions bot added the 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… label Jan 20, 2022

monsieurtanuki requested changes Jan 21, 2022

View reviewed changes

packages/smooth_app/lib/pages/scan/scanner_state_manager.dart Outdated Show resolved Hide resolved

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart Outdated Show resolved Hide resolved

packages/smooth_app/lib/pages/scan/ml_kit_scan_page.dart Outdated Show resolved Hide resolved

Review

740a4f1

M123-dev requested a review from monsieurtanuki January 21, 2022 11:36

monsieurtanuki requested changes Jan 21, 2022

View reviewed changes

jasmeet0817 reviewed Jan 21, 2022

View reviewed changes

monsieurtanuki self-requested a review January 22, 2022 09:53

monsieurtanuki approved these changes Jan 22, 2022

View reviewed changes

monsieurtanuki merged commit 72a2f27 into develop Jan 22, 2022

monsieurtanuki deleted the camera-state-fix branch January 22, 2022 09:54

github-actions bot mentioned this pull request Jan 22, 2022

chore(develop): release 1.0.0 #962

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Camera state management #990

fix: Camera state management #990

M123-dev commented Jan 20, 2022

monsieurtanuki left a comment

M123-dev commented Jan 21, 2022

monsieurtanuki left a comment

jasmeet0817 Jan 21, 2022

M123-dev Jan 21, 2022

jasmeet0817 Jan 21, 2022

M123-dev Jan 21, 2022

jasmeet0817 Jan 22, 2022

jasmeet0817 Jan 21, 2022

M123-dev Jan 21, 2022

jasmeet0817 Jan 22, 2022

jasmeet0817 Jan 21, 2022

M123-dev Jan 21, 2022

jasmeet0817 left a comment

jasmeet0817 commented Jan 21, 2022

jasmeet0817 Jan 21, 2022

M123-dev Jan 21, 2022

M123-dev commented Jan 21, 2022 •

edited

Loading

M123-dev commented Jan 21, 2022

monsieurtanuki left a comment

fix: Camera state management #990

fix: Camera state management #990

Conversation

M123-dev commented Jan 20, 2022

monsieurtanuki left a comment

Choose a reason for hiding this comment

M123-dev commented Jan 21, 2022

monsieurtanuki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmeet0817 left a comment

Choose a reason for hiding this comment

jasmeet0817 commented Jan 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

M123-dev commented Jan 21, 2022 • edited Loading

M123-dev commented Jan 21, 2022

monsieurtanuki left a comment

Choose a reason for hiding this comment

M123-dev commented Jan 21, 2022 •

edited

Loading